Skip to content

Pass num_dim to geoclaw TopographyData and DTopoData - #208

Open
mandli wants to merge 1 commit into
clawpack:masterfrom
mandli:fix-1d-topo-data-format
Open

Pass num_dim to geoclaw TopographyData and DTopoData#208
mandli wants to merge 1 commit into
clawpack:masterfrom
mandli:fix-1d-topo-data-format

Conversation

@mandli

@mandli mandli commented Sep 7, 2026

Copy link
Copy Markdown
Member

This ensures that older versions of clawpack do not break if num_dim is not an argument to TopographyData and DTopoData. The value num_dim is now required in these to support the 1D code.

Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5
@mandli

mandli commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

PR Detailed Description -- Summarized by Claude

Companion: clawpack/geoclaw#745, which adds the num_dim argument and the 1D
write paths. Merge geoclaw#745 first. It defaults to num_dim=2, so it is
inert on its own; this PR landing first against an older geoclaw would
otherwise break every geoclaw setrun. The fallback below covers that case
regardless.

Why this is needed

ClawRunData builds the same TopographyData/DTopoData objects for 1D and
2D runs, but src/1d_classic and src/2d/shallow do not share a Fortran
reader, and the 1D reader was never updated for the per-file preprocessing
block that clawpack/geoclaw#726 introduced. Every examples/1d_classic case
aborts at startup reading topo.data.

Those classes cannot infer the dimension themselves, and
ClawData.__setattr__ rejects unknown attributes, so setting num_dim after
construction is not an option either. Passing it in at construction is the
route already used for the neighbouring objects:

self.add_data(amrclaw.GaugeData(num_dim=num_dim), 'gaugedata')
self.add_data(amrclaw.RegionData(num_dim=num_dim), 'regiondata')
self.add_data(amrclaw.AdjointData(num_dim=num_dim), 'adjointdata')

TopographyData and DTopoData were the odd ones out.

The change

Four call sites needed it — TopographyData and DTopoData, in both the
geoclaw and the dclaw package branches — so rather than repeat a
try/except four times it goes through one helper:

def _make_dim_aware(cls, num_dim):
    try:
        return cls(num_dim=num_dim)
    except TypeError:
        return cls()

dclaw is 2D in practice, but it uses the same geoclaw data classes and
passing num_dim there is correct and harmless.

On the fallback

It exists specifically for mixed clawutil/geoclaw checkouts, which are routine
during development — the 1D breakage was found in exactly that situation, with
several repos on different branches. Without it, a clawutil ahead of geoclaw
turns a TypeError in ClawRunData.__init__ into a failure of every geoclaw
setrun.py, which is a much worse failure than the one being fixed.

It is deliberately temporary and the docstring says so: once the geoclaw
requirement catches up, _make_dim_aware should collapse to a direct call.
Catching TypeError is narrow enough here because neither __init__ raises
TypeError internally — both only call add_attribute.

Verification

Exercised against the geoclaw side of the pair:

  • ClawRunData('geoclaw', 1)topo_data.num_dim == 1 and
    dtopo_data.num_dim == 1.
  • ClawRunData('geoclaw', 2) — both 2; 2D data files byte-identical to
    before.
  • All six examples/1d_classic cases build and run end to end, which is the
    behaviour this argument exists to enable.
  • geoclaw pytest tests/ -m "not remote": 519 passed, 4 skipped, 1 xfailed.
  • geoclaw examples/tsunami/chile2010 (2D, exercises both topo and dtopo):
    6 passed.

The fallback path was confirmed by construction rather than by pinning an old
geoclaw: with geoclaw#745 applied, cls(num_dim=...) succeeds and the
except branch is not taken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant